-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make overflow arithmetic builtins return tuples #14024
Conversation
45332c5
to
a12d57a
Compare
const ov = @addWithOverflow(res.fraction, 1); | ||
res.fraction = ov[0]; | ||
res.exp += ov[1]; | ||
res.fraction |= @as(u64, ov[1]) << 63; // Restore integer bit after carry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignore this if this was already discussed and firmly agreed on as-is internally but like I said in my comment in the issue (#10248 (comment)), a regular struct with named fields would be much easier to read here (e.g. struct { result: T, overflow: u1 }
). Now I'm going to have to memorize what the result value is and what the overflow value is.
These are now the first builtins to my knowledge with this kind of rather not so obvious return type.
If Zig gets destructuring assignment in the future, the situation might improve a bit, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just found that functions like std.math.add
, std.math.mul
, etc are doing exactly this using builtins as the lower-level construct under the hood. So, for safe math one should just use standard lib functions I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those std.math
variants do look like they can be a more readable alternative.
c3d638a
to
462d40d
Compare
This was fixed by 8a0a6b7 for targets without avx512
4a40c07
to
a777373
Compare
Closes #10248
Closes #14022